Skip to content

Conversation

vklachkov
Copy link
Contributor

And rewrite logs with tracing

@vklachkov vklachkov marked this pull request as draft September 12, 2025 18:35
@vklachkov vklachkov marked this pull request as ready for review September 13, 2025 02:29
@vklachkov vklachkov changed the title Add HTTP client HTTP client & tracing Sep 13, 2025
@vklachkov
Copy link
Contributor Author

Сейчас поддержка прогресса сделана через tracing-indicatif. Возможно, стоит ещё прикрутить OSC9 progress для красоты

@CertainLach
Copy link
Member

Возможно, стоит ещё прикрутить OSC9 progress для красоты

Я бы сказал что поддержка со стороны терминалов слишком мала чтобы думать об этом в chainql

Если однако ты добавишь это в indicatif, чтобы оно делало OSC9 для суммарно всех выбранных прогрессбаров (opt-in) - то можно и использовать

@CertainLach
Copy link
Member

В апстриме есть пожелание это иметь: console-rs/indicatif#596

@CertainLach
Copy link
Member

    0: remove progress.
    1: set progress value to pr (number, 0-100).
    2: set the taskbar to the "Error" state
    3: set the taskbar to the "Indeterminate" state
    4: set the taskbar to the "Warning" state

Состояние indicatif пустое (Ни один из spanов не заopt-inился в отображение прогресса) - делаем 0
Состояние indicatif не пустое - делаем indeterminate вне зависимости от прогрессов, потому что не любой прогрессбар имеет смысл тут отображать
Имеется >=1 прогрессбар который заopt-inился в отображение прогресса через osc - суммируем прогресс как сумму дробей, и делаем 1. Если один прогрессбар вложен в другой - то берём только toplevel

Что касаемо error/warning - в indicatif есть возможность удалить прогрессбар с сообщением, и тогда он ещё несколько секунд висит с сообщением... Однако будто бы оно мерцать будет так, и error/warning полезны только для программ с интерактивщиной...

@vklachkov
Copy link
Contributor Author

@CertainLach я поправил по замечаниям. Думаю, можно вливать

header_span.pb_set_message("preloading keys");
header_span.pb_set_finish_message("all keys preloaded");

let header_span_enter = header_span.enter();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Если пользоваться header_span не нужно, то лучше сделать

let _header_span = header_span.entered();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Как я вижу этот span не используется для логов, а поэтому лучше в него даже не входить явно тут, используй Future::instrument для тела block_on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не понимаю. Мне же нужно не просто войти в спан, но и прописать "indicatif.pb_show = true" и засетапить текст и стили

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instrument делает автоматический вход в span при каждом poll Future

https://docs.rs/tracing/latest/tracing/span/struct.Span.html#in-asynchronous-code

Технически у нас тут block_on работает как await, а соответственно это к нему применимо

.query_storage(list, Some(self.block.as_ref()))
.await?;

Span::current().pb_inc(chunk_size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Лучше тут явно span использовать (через аргументы принимать, возможно Arc, особенно учитывая как они работают с async-await (для Futures надо делать future.instrument кроме .enter())

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tokio может свои spanы для pollа добавлять, например если tokio-console включить

instrument() решает эту проблему, однако всё равно это выглядит неявно, pb_inc лучше на конкретном дёргать

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants